Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(base-zone,zone): import isPassable from @endo/pass-style #9230

Merged
merged 3 commits into from
Apr 30, 2024

Conversation

erights
Copy link
Member

@erights erights commented Apr 14, 2024

closes: #XXXX
refs: endojs/endo#2096 endojs/endo#2042

Description

Deleting @agoric/base-zone's implementation of isPassable completes the migration of isPassable from @agoric/base-zone to @endo/pass-style explained in endojs/endo#2096 and started in endojs/endo#2042

The remaining issue explained in endojs/endo#2096 , changing how isPassable is implemented, remains to be done in @endo/pass-style. But this need no longer concern us here since that difference will now be encapsulated from us.

Security Considerations

None

Scaling Considerations

We know that passStyleOf remains a performance hotspot that needs attention. This PR does not affect that at all. But I'll note that the remaining suggested change from endojs/endo#2096 --- to implement isPassable and passStyleOf in terms of a more expressive internal function parameterized by a checker --- might make this performance issue worse. Just something to keep in mind as we tune passStyleOf. Attn @gibson042

Documentation Considerations

none

Testing Considerations

none

Upgrade Considerations

As of this PR, @agoric/base-zone also no longer exports isPassable, potentially breaking importers outside agoric-sdk until they are modified to import it from @endo/pass-style as well. This PR does take care of all such import sites within agoric-sdk.

If this turns out to be a problem in practice, this PR could be changed to have @agoric/base-zone reexport the isPassable it imports from @endo/pass-style, but deprecate that reexport, leaving it to future work to change those old import sites outside agoric-sdk.

@erights erights self-assigned this Apr 14, 2024
Copy link

cloudflare-workers-and-pages bot commented Apr 14, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 979fe17
Status: ✅  Deploy successful!
Preview URL: https://f300c455.agoric-sdk.pages.dev
Branch Preview URL: https://markm-delegate-is-passable.agoric-sdk.pages.dev

View logs

@erights erights marked this pull request as ready for review April 14, 2024 21:10
@erights erights changed the title fix(base-zone,zone): import isPassable from @endo/pass-style fix(base-zone,zone): import isPassable from @endo/pass-style Apr 14, 2024
Copy link
Member

@michaelfig michaelfig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a way to mark a conventional commit as a breaking change (note that the parenthesised portion of the commit message cannot contain commas):

fix(base-zone)!: do not export `isPassable`; it is available from `@endo/pass-style`

packages/base-zone/src/is-passable.js Show resolved Hide resolved
packages/base-zone/src/index.js Outdated Show resolved Hide resolved
@erights
Copy link
Member Author

erights commented Apr 28, 2024

PTAL, thanks

@erights erights added the automerge:squash Automatically squash merge label Apr 30, 2024
@mergify mergify bot merged commit fbd8633 into master Apr 30, 2024
67 checks passed
@mergify mergify bot deleted the markm-delegate-is-passable branch April 30, 2024 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:squash Automatically squash merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants